feat: implement PriceCalculator rule for Epic 5.4#145
Conversation
Detailed TDD plan with 6 tasks: - Task 1: Quality multiplier tests (6 tiers) - Task 2: Supply/demand modifier tests - Task 3: Combined modifiers tests - Task 4: Edge case tests - Task 5: Rounding behavior tests - Task 6: Full verification Expected: 22 tests total Refs #43
Implements 6-tier quality multiplier system from GDD: - Crude (0-20): 0.5x - Common (21-40): 1.0x - Quality (41-60): 1.5x - Fine (61-80): 2.0x - Superior (81-99): 3.0x - Masterwork (100+): 5.0x Refs #43
Tests verify 4 market conditions: - Abundant (0.5x) - Normal (1.0x) - Scarce (1.5x) - Very Rare (2.0x) Refs #43
Tests verify realistic pricing with both quality and supply/demand: - Fine quality + scarce market - Crude quality + abundant market - Masterwork + abundant market Refs #43
Tests verify boundary conditions and error handling: - Quality at 0 and > 100 - Zero base price - Negative quality (throws exception) - Zero/negative supply/demand (throws exception) Refs #43
Tests verify: - Fractional copper amounts round correctly - Large values don't overflow - Standard Math.Round() behavior Refs #43
Recommended Persona Reviews💻 .NET Specialist - C# code changes detected See docs/roles/ for persona details. Danger analysis complete for feat: implement PriceCalculator rule for Epic 5.4. 6 files changed, 9 commits. |
mcj-coder
left a comment
mcj-coder
left a comment
There was a problem hiding this comment.
Code Review - Approved
Reviewed as .NET 10 Specialist persona.
Summary
Status: APPROVED ✅
This is high-quality .NET code that demonstrates excellent craftsmanship. The implementation is production-ready, well-tested, and follows all established patterns. All 22 tests pass, CI checks are green.
Strengths
✅ Pattern Consistency - Perfectly matches XpCalculator and CraftingOutcomeRule patterns
✅ Type Safety - Excellent use of Currency value object for type-safe price handling
✅ Validation - Comprehensive input validation with clear error messages
✅ Modern C# - Clean switch expressions and appropriate use of C# 14 features
✅ Mathematical Correctness - Proper use of decimal for calculations, Math.Round() for rounding
✅ Test Quality - 22 comprehensive tests covering all scenarios (quality tiers, supply/demand, edge cases, rounding)
✅ Documentation - Clear XML docs and inline comments
Minor Notes (Non-Blocking)
- Minor path discrepancy in PR description (references
FantasyRpgWorld.Simulation/Rules/but actual path isFantasyRpgWorld.Core/Rules/) - Rounding mode uses default banker's rounding (ToEven) vs XpCalculator's AwayFromZero - both are valid, but could consider explicit mode for clarity
These are documentation notes only and do not affect code quality.
Technical Assessment
| Criterion | Rating |
|---|---|
| C# Best Practices | ⭐⭐⭐⭐⭐ Excellent |
| .NET 10 Compatibility | ⭐⭐⭐⭐⭐ Excellent |
| Type Safety | ⭐⭐⭐⭐⭐ Excellent |
| Test Coverage | ⭐⭐⭐⭐⭐ Excellent |
| Pattern Consistency | ⭐⭐⭐⭐⭐ Excellent |
Conclusion
Production-ready code. Ready to merge.
Reviewed by: Martin Jarvis (Tech Lead)
2 participants
Summary
Implements the PriceCalculator rule that calculates item prices based on:
Formula: Final Price = Base Price × Quality Multiplier × Supply/Demand Multiplier
Issue
Closes #43
Changes
Testing
All tests passing: